Skip to content

Change OdbBackend.ReadPrefix() signature#589

Merged
nulltoken merged 2 commits into
vNextfrom
ntk/topic/odbbackend_readprefix
Dec 21, 2013
Merged

Change OdbBackend.ReadPrefix() signature#589
nulltoken merged 2 commits into
vNextfrom
ntk/topic/odbbackend_readprefix

Conversation

@nulltoken

@nulltoken nulltoken commented Dec 20, 2013

Copy link
Copy Markdown
Member

From

public abstract int ReadPrefix(
  byte[] shortOid,
  int prefixLen,
  out byte[] oid,
  out Stream data,
  out ObjectType objectType);

This should ease (a little bit) the task of implementing a backend

To

public abstract int ReadPrefix(
  string shortSha,
  out ObjectId id,
  out Stream data,
  out ObjectType objectType);

@nulltoken

nulltoken commented Dec 19, 2013

Copy link
Copy Markdown
Member Author

/cc @phkelley

@phkelley

phkelley commented Dec 19, 2013

Copy link
Copy Markdown
Member

Hi! This sounds like a great change. It looks like it would be LibGit2Sharp-only, too.

@nulltoken

nulltoken commented Dec 20, 2013

Copy link
Copy Markdown
Member Author

@phkelley I've pushed a proposal. Could you please review?

phkelley
phkelley reviewed Dec 20, 2013
View reviewed changes
Comment thread LibGit2Sharp/ObjectId.cs Outdated

@phkelley phkelley Dec 20, 2013

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might rename this variable to lengthInNibbles to provide clarity.

@nulltoken nulltoken Dec 21, 2013

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@phkelley

phkelley commented Dec 20, 2013

Copy link
Copy Markdown
Member

I left some comments, @nulltoken. Thanks for doing this!

@nulltoken

nulltoken commented Dec 21, 2013

Copy link
Copy Markdown
Member Author

Cool to merge, now?

@phkelley

phkelley commented Dec 21, 2013

Copy link
Copy Markdown
Member

I left one more note - I should have caught it earlier. We are still invoking StartsWith on a String in the test code, instead of on an ObjectId. That is to say, the new helper method you wrote on ObjectId is not being used yet. But after that I would say it is good to go!! Thanks so much

@nulltoken

nulltoken commented Dec 21, 2013

Copy link
Copy Markdown
Member Author

Cool to merge, now?

@phkelley @dahlbyk How about now?

@dahlbyk

dahlbyk commented Dec 21, 2013

Copy link
Copy Markdown
Member

👍 from me

@nulltoken nulltoken merged commit c405b0f into vNext Dec 21, 2013
@nulltoken nulltoken deleted the ntk/topic/odbbackend_readprefix branch December 21, 2013 22:00
@nulltoken

nulltoken commented Dec 21, 2013

Copy link
Copy Markdown
Member Author

Merged. Thanks guys for all the help! 😍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Up for grabs

Projects

None yet

Milestone

v0.15.0

Development

Successfully merging this pull request may close these issues.

3 participants

@nulltoken @phkelley @dahlbyk